fix(service-automation): compare the screen caller-provenance record leg by value, so a durable resume cannot skip a screen - #16392

Fixes #15812

The card reproduces end to end against a wired store

That was the first job here, and the outcome was not preordained — "does not reproduce" would have been a perfectly good delivery. It reproduces.

The rig satisfies every clause of the card's conjunction at once, none stubbed: the actions door (so seedFlowActionParams' { ...record, recordId, the camelCase objectName-plus-Id alias, ...params } spreads the row into params), a wired durable store (InMemorySuspendedRunStore, which JSON-clones on save and on load, the same boundary sys_automation_run's context_json imposes), a later screen in the same run entered after the resume, a non-primitive colliding column (tags: ['a','b']), and no other required field on that screen to force the pause anyway.

Measured on the unmodified tree at 0374bcba9, driving a real suspend then a real resume:

PROBE first screen status= paused node= collect
PROBE resumed status= undefined screen= undefined success= true
      output= {"full_name":"Ada","tags":["a","b"]}

The review screen was skipped, and the run completed carrying the row's own tags as if a caller had typed it. A human pressed a button and never saw a form they should have been shown — the failure direction #15705 exists to prevent.

One correction to the card's own mechanism note, worth recording because it widens reachability: this needs no process restart. loadSuspendedRunStrict prefers the store over the hot cache whenever one is wired (if (!this.store) return this.suspendedRuns.get(runId)), so any resume on a store-backed engine judges against the parsed copy.

The change

One predicate, in callerSupplied's record leg:

return !isDeepStrictEqual(value, record[name]);   // was: !Object.is(...)

isDeepStrictEqual compares primitives with Object.is itself (verified: 0/-0 unequal, NaN equal, no cross-type coercion), so this is a strict widening of the old predicate — every pair identity called equal it still calls equal, plus the structurally identical non-primitives. The widened set is "not caller-supplied", i.e. more pauses. The safe direction is therefore a property of the substitution, not a claim about it.

⛔ The row-id leg deliberately keeps Object.is: a row id is a scalar by construction (params.recordId is seeded as one, record.id is one), so serialisation cannot defeat it and there is nothing there to widen. That decision is pinned by a control, not left as prose.

The two trade-offs, answered head-on

1. Cost — with magnitude, measured

Per-comparison, on structurally equal values that are not identical (the round trip's output, and the worst case for a deep compare since it must walk the whole structure to say "equal"), 200,000 iterations each:

Per screen entry, which is the unit that matters — the comparison runs at most once per declared field whose name is present in params. A deliberately maximal screen that declares a field for every one of a ten-column row (string, array, nested object, 50-element multiselect, scalars):

record leg, Object.is                227 ns
record leg, isDeepStrictEqual       1700 ns   (+1474 ns)
JSON.stringify(context) — already paid per suspend:  3913 ns
added cost as a fraction of that one existing serialisation:  37.7%

So the added work is ~1.5 µs on a maximal screen, against a code path that already spends ~3.9 µs serialising the same context on every suspend, at an event that is a user-facing form entry rather than a hot loop. ⚠️ Shared-box readings — this container runs parallel agents; the ratios are the durable half.

The unbounded tail is real and stated rather than waved away: the last table row is 13 µs for a blob nothing in a screen field should be, and a deep compare is O(size). It is bounded by the same value the store already stringifies on every suspend, so it cannot be asymptotically worse than the persistence this leg exists to survive.

2. Semantic widening — and why the direction is the safe one

Value equality enlarges "not caller-supplied": a caller that genuinely re-sends a value structurally identical to the row's moves from continue to pause. Under Object.is two distinct-but-equal arrays read as caller-supplied and the screen was skipped; now they are indistinguishable from the dispatcher's seed and it renders.

That is a lost skip on a headless call, never a lost run, and it is the direction this module resolves every ambiguity in — condition 1 of judgeHeadlessScreen refuses on every uncertainty, the row-id legs refuse a value merely equal to the row id, and an unevaluable visibleWhen keeps the screen interactive. Landing this one anywhere else would make the module's newest leg the only one that resolves doubt toward skipping a screen a human should see. It is pinned as a behaviour change (WIDENING — below), not left to be discovered.

Tests

packages/services/service-automation/src/builtin/screen-headless-provenance-durable-resume.test.ts — six cases, and the controls outnumber the bug deliberately, because the pause the fixed code produces has to be attributable to this leg:

The last two are the ones that stop a false positive: the primitive control passes on both sides, so it can never be misread as evidence of the fix, and the "different value" case is what keeps the fix from "passing" by pausing everything.

Red before green. The prediction was written before the first run: cases 1 and 5 red, four green. Measured on the unmodified tree — Tests 2 failed | 4 passed (6), exactly those two, both expected undefined to be 'paused'.

Ablation, from the committed implementation at 32b008c5e. The record leg was reverted to Object.is; the mutation was confirmed on disk before the run (injected-marker count 1, deleted-marker count 0, blob hash moved off c2f87e13), giving Tests 2 failed | 4 passed (6) — the same two cases, every control still green. A test resolving a stale dist would have stayed green here, so the red is also the proof the mutation was read. The restore leg was proven three ways rather than trusted to its trap: git checkout HEAD -- (absolute path), then blob hash equal to the HEAD blob (c2f87e13704d36d9445f800e0a4c3e117fe52d3f), git diff HEAD empty, and marker counts back at 1/0.

Verification at the final head 2e5899656:

• pnpm --filter @objectstack/service-automation test — 122 files, 1432 tests, all passing
• pnpm --filter @objectstack/service-automation typecheck — clean, check:test-typecheck OK. It first went red on the new test file (TS2339: Property 'tags' does not exist on type '{}'), which is the direct evidence that this package's test layer really is type-checked rather than excluded.
• the gate family derived mechanically at that head — node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands, 57 commands, all run, all green. Two returned exit 3 PREREQUISITE NOT MET (check:dual-build-cjs-loads, check:type-check-debt) — neither pass nor finding; the closure was built and both re-run to real green readings.
• pnpm lint — the whole-repo eslint . --no-inline-config, green (80 s). Run in full, so nothing here rests on a narrowing.

Scope

.changeset is a patch on @objectstack/service-automation, not skip-changeset: the package publishes from dist, and this changes a runtime disposition a flow author and an end user can both observe. It also names the gap that release's screen-flow headless-satisfaction note records as known, so the compiled notes do not contradict themselves.

Clause-② holds as declared: packages/spec/** is untouched and no exported signature moves. screen-input-contract.ts is not re-exported from the package index — judgeHeadlessScreen and callerSupplied are package-internal, and nothing outside this package references either.

⛔ Boundaries kept: this is not a defect in PR #15787 and not an argument against it — that provenance check is what makes headless screen satisfaction safe at all, and nothing here reverts or weakens it. Out of scope: the dispatcher-seeded id keys that PR handles internally, and #15646's pausing-map residue, which is the opposite direction (state outliving what wrote it, rather than identity failing to survive).

⛔ Not widened into option B (an explicit caller-provenance signal on AutomationContext) — a spec + runtime contract change the PM scoped out deliberately. But the #15705 dev's observation belongs on the record, because it describes the shape accurately:

B is the only option that removes the CLASS rather than its instances — every leg in this module is an inference about what the caller meant, and each new dispatcher seed re-opens it, which is precisely how X1 happened.

This change closes an instance. The module now has three legs, all of them inferences about what the caller meant, and the count is a function of how many things the dispatch doors put in params without saying so. If a third dispatcher seed ever forces a fourth leg, that is the signal the class rather than the instance is what needs closing, and B is where it goes.

🤖 Generated with Claude Code

https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y

Generated by Claude Code